I would naively assume upon reading this conditional that either the addition, or direction of the inequality, was wrong. Or maybe that it should be if (first + number_of_potential_values > length) { fail } instead? Could it not be the case that a file uses start = 200 and length 256 and would still be valid for an 8bpp image - assuming that the image never used values > 56?

Your question forced me to admit that my understanding here was shaky, so I did some research and I think our code is wrong (and always has been). Here's what I learned, assuming I understand it correctly now.

As you probably know, TARGA files come from Truevision, who made early PC frame buffers / accelerators. The hardware palette registers were a shared resource, and often the OS or other hardware features wanted to reserve the first 16 or whatever and not have those change, nor store them in the file. The 'start' value is the destination offset into a hardware LUT where your palette should be copied.

Near as I can tell -- and contrary to our buggy code! -- the indices in the image pixels are absolute, not relative to the palette's start value. So if the (first+length) is more than can be addressed by the bpp, something has gone awry, because those high indices can never be specified in the pixels.

It seems that our current code in decode_pixel is wrong, it's adding the indices in the file to the start offset, but I now believe that it shouldn't be doing that. In fact, since we only allocate and store the ones beginning with 'start', what we should be doing is subtracting the start value to index into our own array.

Of course, it's been decades since Truevision boards, so anybody making TGA files is not worried about overwriting reserved LUT entries in the hardware, and thus there is no reason to reserve palette value and have the start value be anything other than 0. This is probably why we've never discovered this issue before.

I will revise to fix these bugs as well.

My copy of the Targa spec actually doesn't say whether the pixel values are absolute or relative indices. But I spot-checked some other software to see what they did, and it seems they all treat it as absolute. I suppose the only files you'd find with a nonzero start value are ones that date from the era where people were intending the image to run on a specific hardware board (because the number of palette registers you'd want to not disturb wasn't the same for all board models), and that's the reason we never came across this condition in practice.